Skip to content

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented May 15, 2025

  • define the RootDescriptor in-memory struct containing its type
  • add test harness for testing

First part of #126577

- define the RootParam in-memory struct containing its type
- add test harness for testing
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels May 15, 2025
@llvmbot
Copy link
Member

llvmbot commented May 15, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Finn Plummer (inbelic)

Changes
  • define the RootParam in-memory struct containing its type
  • add test harness for testing

First part of #126577


Full diff: https://github.com/llvm/llvm-project/pull/140147.diff

4 Files Affected:

  • (modified) clang/include/clang/Parse/ParseHLSLRootSignature.h (+1)
  • (modified) clang/lib/Parse/ParseHLSLRootSignature.cpp (+43)
  • (modified) clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp (+37)
  • (modified) llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h (+9-5)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 915266f8a36ae..b96416ab6c6c4 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -73,6 +73,7 @@ class RootSignatureParser {
   /// Root Element parse methods:
   std::optional<llvm::hlsl::rootsig::RootFlags> parseRootFlags();
   std::optional<llvm::hlsl::rootsig::RootConstants> parseRootConstants();
+  std::optional<llvm::hlsl::rootsig::RootParam> parseRootParam();
   std::optional<llvm::hlsl::rootsig::DescriptorTable> parseDescriptorTable();
   std::optional<llvm::hlsl::rootsig::DescriptorTableClause>
   parseDescriptorTableClause();
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 5603900429844..3ed60442eaa14 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -47,6 +47,14 @@ bool RootSignatureParser::parse() {
         return true;
       Elements.push_back(*Table);
     }
+
+    if (tryConsumeExpectedToken(
+            {TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) {
+      auto RootParam = parseRootParam();
+      if (!RootParam.has_value())
+        return true;
+      Elements.push_back(*RootParam);
+    }
   } while (tryConsumeExpectedToken(TokenKind::pu_comma));
 
   return consumeExpectedToken(TokenKind::end_of_stream,
@@ -155,6 +163,41 @@ std::optional<RootConstants> RootSignatureParser::parseRootConstants() {
   return Constants;
 }
 
+std::optional<RootParam> RootSignatureParser::parseRootParam() {
+  assert((CurToken.TokKind == TokenKind::kw_CBV ||
+          CurToken.TokKind == TokenKind::kw_SRV ||
+          CurToken.TokKind == TokenKind::kw_UAV) &&
+         "Expects to only be invoked starting at given keyword");
+
+  TokenKind ParamKind = CurToken.TokKind;
+
+  if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+                           CurToken.TokKind))
+    return std::nullopt;
+
+  RootParam Param;
+  switch (ParamKind) {
+  default:
+    llvm_unreachable("Switch for consumed token was not provided");
+  case TokenKind::kw_CBV:
+    Param.Type = ParamType::CBuffer;
+    break;
+  case TokenKind::kw_SRV:
+    Param.Type = ParamType::SRV;
+    break;
+  case TokenKind::kw_UAV:
+    Param.Type = ParamType::UAV;
+    break;
+  }
+
+  if (consumeExpectedToken(TokenKind::pu_r_paren,
+                           diag::err_hlsl_unexpected_end_of_params,
+                           /*param of=*/TokenKind::kw_RootConstants))
+    return std::nullopt;
+
+  return Param;
+}
+
 std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() {
   assert(CurToken.TokKind == TokenKind::kw_DescriptorTable &&
          "Expects to only be invoked starting at given keyword");
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index c97f8d0b392d1..876dcd17f0389 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -344,6 +344,43 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) {
   ASSERT_TRUE(Consumer->isSatisfied());
 }
 
+TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) {
+  const llvm::StringLiteral Source = R"cc(
+    CBV(),
+    SRV(),
+    UAV()
+  )cc";
+
+  TrivialModuleLoader ModLoader;
+  auto PP = createPP(Source, ModLoader);
+  auto TokLoc = SourceLocation();
+
+  hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+  SmallVector<RootElement> Elements;
+  hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+  // Test no diagnostics produced
+  Consumer->setNoDiag();
+
+  ASSERT_FALSE(Parser.parse());
+
+  ASSERT_EQ(Elements.size(), 3u);
+
+  RootElement Elem = Elements[0];
+  ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
+  ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::CBuffer);
+
+  Elem = Elements[1];
+  ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
+  ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::SRV);
+
+  Elem = Elements[2];
+  ASSERT_TRUE(std::holds_alternative<RootParam>(Elem));
+  ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::UAV);
+
+  ASSERT_TRUE(Consumer->isSatisfied());
+}
+
 TEST_F(ParseHLSLRootSignatureTest, ValidTrailingCommaTest) {
   // This test will checks we can handling trailing commas ','
   const llvm::StringLiteral Source = R"cc(
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 9fdb40db9c23d..2bcae22c6cfb2 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -85,6 +85,11 @@ struct RootConstants {
   ShaderVisibility Visibility = ShaderVisibility::All;
 };
 
+using ParamType = llvm::dxil::ResourceClass;
+struct RootParam {
+  ParamType Type;
+};
+
 // Models the end of a descriptor table and stores its visibility
 struct DescriptorTable {
   ShaderVisibility Visibility = ShaderVisibility::All;
@@ -125,8 +130,8 @@ struct DescriptorTableClause {
   void dump(raw_ostream &OS) const;
 };
 
-/// Models RootElement : RootFlags | RootConstants | DescriptorTable
-///  | DescriptorTableClause
+/// Models RootElement : RootFlags | RootConstants | RootParam
+///  | DescriptorTable | DescriptorTableClause
 ///
 /// A Root Signature is modeled in-memory by an array of RootElements. These
 /// aim to map closely to their DSL grammar reprsentation defined in the spec.
@@ -140,9 +145,8 @@ struct DescriptorTableClause {
 /// The DescriptorTable is modelled by having its Clauses as the previous
 /// RootElements in the array, and it holds a data member for the Visibility
 /// parameter.
-using RootElement = std::variant<RootFlags, RootConstants, DescriptorTable,
-                                 DescriptorTableClause>;
-
+using RootElement = std::variant<RootFlags, RootConstants, RootParam,
+                                 DescriptorTable, DescriptorTableClause>;
 void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements);
 
 class MetadataBuilder {

@inbelic inbelic linked an issue May 16, 2025 that may be closed by this pull request
4 tasks
@inbelic inbelic force-pushed the inbelic/rs-empty-root-param branch from 182d1af to cddbc9a Compare May 21, 2025 18:54
@inbelic inbelic changed the title [HLSL][RootSignature] Add parsing for empty RootParams [HLSL][RootSignature] Add parsing for empty RootDescriptors May 21, 2025
@inbelic inbelic merged commit b499f7f into llvm:main May 21, 2025
12 checks passed
@inbelic inbelic deleted the inbelic/rs-empty-root-param branch June 2, 2025 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HLSL] Implement Root Signature Parsing of Root Parameters
4 participants